Issue 924: add support for arrays in scientificMetadata #925
Conversation
|
Hi, |
|
I've just added a workaround for #926 |
| inputUnit: string, | ||
| ): { valueSI: number[]; unitSI: string } => { | ||
| try { | ||
| const newUnit = unit(inputUnit).toSI().toJSON().unit; |
There was a problem hiding this comment.
can add a comment for this workaround, so that we can easier find to roll back these changes once upstream mathjs has fixed that behaviour ( and if possible reference a mathjs bug report as well)
There was a problem hiding this comment.
#1005 bumped to version with fixed mathjs (see https://github.com/josdejong/mathjs/blob/develop/HISTORY.md#2024-01-12-1230) so we could drop the workaround now.
There was a problem hiding this comment.
This looks fine to me, although it does not address the following:
- API tests to test specifically this feature
- fullquery logic and syntax to deal with this new type of metadata.
- UI to view and edit this type of metadata (this should be a FE PR)
Potentially we could merge this PR, as long as we start working right away on the other two tasks right.
@Junjiequan and myself just discussed about this and we think that it should be discussed at the next meeting.
|
Please check the frontend edit function as well ( it currently seems to truncate the scientific metadata if it encounters an array in the values)! |
I think the frontend has a separate repository so one need to open a ticket in the dedicated repository |
|
What's the status of this PR? |
Description
It resolves #924 by adding support for arrays in scientificMetadata
Motivation
At DESY we often use a list to store vectors
Changes:
convertArrayToSIfunction which is called byappendSIUnitToPhysicalQuantity